Add electrum support #131

dr-orlovsky · 2024-02-13T15:28:20Z

These are electrum-specific commits extracted from v0.11 branch #103, including the commit from #126 PR, which I move here to be able to run CI on the whole thing.

We need to make sure this compiles to WASM before merging - and I expect it should due to BP-WG/bp-wallet@9d1cf14, making #127 and #128 outdated.

I will wait for @crisdut and @zoedberg confirmation before merging this and close the other two PRs trying to fix default features of bp-wallet

zoedberg · 2024-02-13T15:46:10Z

it works good, thanks

crisdut · 2024-02-13T19:35:02Z

I tried compile for wasm target and I receive some errors:

wasm-pack build --no-default-features --features serde

First Error : AnyResolver

error[E0004]: non-exhaustive patterns: type `&AnyResolver` is non-empty
  --> src/resolvers/any.rs:77:15
   |
77 |         match self {
   |               ^^^^
   |
note: `AnyResolver` defined here
  --> src/resolvers/any.rs:34:10
   |
34 | pub enum AnyResolver {
   |          ^^^^^^^^^^^
   = note: the matched value is of type `&AnyResolver`
   = note: references are always considered inhabited
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern as shown
   |
77 ~         match self {
78 +             _ => todo!(),
79 +         }
   |

Regards the blocking http calls, both options in AnyResolver does not working in wasm target.

We can work with a "Dummy" option that would be great for doing some tests without accessing external data, for example:

#[derive(From)]
#[non_exhaustive]
pub enum AnyResolver<T> 
where
    T: ResolveWitness + ResolveHeight + std::fmt::Debug,
{
    #[cfg(feature = "electrum")]
    #[from]
    /// Electrum resolver
    Electrum(Box<electrum::Resolver>),
    #[cfg(feature = "esplora_blocking")]
    #[from]
    /// Esplora resolver
    Esplora(Box<esplora_blocking::Resolver>),
    /// Dummy resolver
    Dummy(T),
}

Second Error : cli as default-member

    |
397 |         let len = self.socket.readv([&mut header, buf])?;
    |                               ^^^^^ method not found in `UdpSocket`
    |
    = help: items from traits can only be used if the trait is implemented and in scope
note: `WritevExt` defines an item `readv`, perhaps you need to implement it
   --> /Users/crisdut/.cargo/registry/src/index.crates.io-6f17d22bba15001f/socks-0.3.4/src/writev.rs:4:1
    |
4   | pub trait WritevExt {
    | ^^^^^^^^^^^^^^^^^^^

If we remove "cli" from default-members, in Cargo.toml, the compilation works.

default-members = [
    "psbt",
    "fs",
    "."
]

cryptoquick · 2024-02-13T19:42:30Z

@crisdut Notice that you conditionally compile it without either esplora or electrum feature, as you have, the match arms would be empty:

https://github.com/RGB-WG/rgb/pull/131/files#diff-7930ac95a70feec3b3c3ff5c75d402c65de5c29c46f784d9e2bc95da0e5ba3d8R78-R81

Perhaps it would make sense to just mark it as non-exhaustive:
#[non_exhaustive]
See: https://doc.rust-lang.org/beta/reference/attributes/type_system.html#the-non_exhaustive-attribute

dr-orlovsky · 2024-02-15T07:58:55Z

Yeah, that's why I suggested making AnyResolver non-exhaustive here: #126 (comment)

Also, this is the reason I was against this approach: #126 (review)

I do not remember all the reasons, but when we tried it in rust-bitcoin, it was a bag of pain in all downstream dependencies, so I wonder if we should go this route...

zoedberg · 2024-02-15T09:39:54Z

By running the command that @crisdut is giving (wasm-pack build --no-default-features --features serde) I receive the following:

Error: crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your Cargo.toml file:

[lib]
crate-type = ["cdylib", "rlib"]
Caused by: crate-type must be cdylib to compile to wasm32-unknown-unknown. Add the following to your Cargo.toml file:

[lib]
crate-type = ["cdylib", "rlib"]

By adding the crate-type as suggested I receive the following errors:

error[E0599]: the method `fill` exists for struct `SystemRandom`, but its trait bounds were not satisfied
   --> /home/zoe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.21.10/src/rand.rs:8:10
    |
7   | /     SystemRandom::new()
8   | |         .fill(bytes)
    | |         -^^^^ method cannot be called on `SystemRandom` due to unsatisfied trait bounds
    | |_________|
    | 
    |
   ::: /home/zoe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ring-0.17.7/src/rand.rs:112:1
    |
112 |   pub struct SystemRandom(());
    |   -----------------------
    |   |
    |   doesn't satisfy `SystemRandom: SecureRandom`
    |   doesn't satisfy `SystemRandom: ring::rand::sealed::SecureRandom`
    |
    = note: the following trait bounds were not satisfied:
            `SystemRandom: ring::rand::sealed::SecureRandom`
            which is required by `SystemRandom: SecureRandom`

error[E0277]: the trait bound `SystemRandom: ring::rand::sealed::SecureRandom` is not satisfied
  --> /home/zoe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.21.10/src/kx.rs:32:86
   |
32 |             ring::agreement::EphemeralPrivateKey::generate(skxg.agreement_algorithm, &rng).ok()?;
   |                                                                                      ^^^^ the trait `ring::rand::sealed::SecureRandom` is not implemented for `SystemRandom`
   |
   = help: the following other types implement trait `ring::rand::sealed::SecureRandom`:
             FixedByteRandom
             FixedSliceRandom<'_>
             FixedSliceSequenceRandom<'_>
   = note: required for `SystemRandom` to implement `SecureRandom`
   = note: required for the cast from `&SystemRandom` to `&dyn SecureRandom`

error[E0277]: the trait bound `SystemRandom: ring::rand::sealed::SecureRandom` is not satisfied
   --> /home/zoe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.21.10/src/sign.rs:196:34
    |
196 |             .sign(self.encoding, &rng, message, &mut sig)
    |                                  ^^^^ the trait `ring::rand::sealed::SecureRandom` is not implemented for `SystemRandom`
    |
    = help: the following other types implement trait `ring::rand::sealed::SecureRandom`:
              FixedByteRandom
              FixedSliceRandom<'_>
              FixedSliceSequenceRandom<'_>
    = note: required for `SystemRandom` to implement `SecureRandom`
    = note: required for the cast from `&SystemRandom` to `&dyn SecureRandom`

error[E0277]: the trait bound `SystemRandom: ring::rand::sealed::SecureRandom` is not satisfied
   --> /home/zoe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.21.10/src/sign.rs:232:50
    |
232 |         EcdsaKeyPair::from_pkcs8(sigalg, &der.0, &rng)
    |                                                  ^^^^ the trait `ring::rand::sealed::SecureRandom` is not implemented for `SystemRandom`
    |
    = help: the following other types implement trait `ring::rand::sealed::SecureRandom`:
              FixedByteRandom
              FixedSliceRandom<'_>
              FixedSliceSequenceRandom<'_>
    = note: required for `SystemRandom` to implement `SecureRandom`
    = note: required for the cast from `&SystemRandom` to `&dyn SecureRandom`

error[E0277]: the trait bound `SystemRandom: ring::rand::sealed::SecureRandom` is not satisfied
   --> /home/zoe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.21.10/src/sign.rs:234:78
    |
234 |             .or_else(|_| Self::convert_sec1_to_pkcs8(scheme, sigalg, &der.0, &rng))
    |                                                                              ^^^^ the trait `ring::rand::sealed::SecureRandom` is not implemented for `SystemRandom`
    |
    = help: the following other types implement trait `ring::rand::sealed::SecureRandom`:
              FixedByteRandom
              FixedSliceRandom<'_>
              FixedSliceSequenceRandom<'_>
    = note: required for `SystemRandom` to implement `SecureRandom`
    = note: required for the cast from `&SystemRandom` to `&dyn SecureRandom`

error[E0277]: the trait bound `SystemRandom: ring::rand::sealed::SecureRandom` is not satisfied
   --> /home/zoe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-0.21.10/src/sign.rs:317:19
    |
317 |             .sign(&rng, message)
    |                   ^^^^ the trait `ring::rand::sealed::SecureRandom` is not implemented for `SystemRandom`
    |
    = help: the following other types implement trait `ring::rand::sealed::SecureRandom`:
              FixedByteRandom
              FixedSliceRandom<'_>
              FixedSliceSequenceRandom<'_>
    = note: required for `SystemRandom` to implement `SecureRandom`
    = note: required for the cast from `&SystemRandom` to `&dyn SecureRandom`

so maybe to build on wasm we need to patch some dependencies differently?

Anyway, when implementing the AnyResolver I was not considering that someone could need building the library with nor electrum nor esplora features. The fix for this should be simple, just protect the any module and the AnyResolver like so:

#[cfg(feature = "electrum")]
#[cfg(feature = "esplora_blocking")]
mod any;

#[cfg(feature = "electrum")]
#[cfg(feature = "esplora_blocking")]
pub use any::AnyResolver;

cryptoquick · 2024-02-15T15:16:44Z

I think I figured out what was keeping this from compiling to WASM. See #132

crisdut · 2024-02-15T14:02:04Z

Cargo.toml

@@ -66,13 +68,15 @@ name = "rgb_rt"
 [dependencies]
 amplify = { workspace = true }
 baid58 = { workspace = true }
+bitcoin = { workspace = true, optional = true }
 commit_verify = { workspace = true }
 strict_types = { workspace = true }
 bp-core = { workspace = true }
 bp-std = { workspace = true }
 bp-wallet = { workspace = true, features = ["fs"] }


Hi @zoedberg,

Please set default-features=false, to avoid load bp-esplora blocking client along wasm compilation.

Suggested change

bp-wallet = { workspace = true, features = ["fs"] }

bp-wallet = { workspace = true, default-features = false, features = ["fs"] }

Hi @crisdut, sorry for the late answer but I've somehow missed this. Thanks for the tip, unfortunately though I'm still receiving the previously reported errors.

zoedberg · 2024-02-23T11:32:16Z

I think I figured out what was keeping this from compiling to WASM. See #132

Nice, so it seems #132, which is using my suggested fix, has fixed the wasm compilation (I've tried the branch and I can confirm it). So @dr-orlovsky, can we merge that PR into this and then this PR into master?

WASM fixes.

dr-orlovsky and others added 4 commits February 13, 2024 16:20

resolver: rename esplora into esplora_blocking and do a dedicated mod

328dcae

add electrum Resolver

ca2b84d

fix electrum Resolver

e5422be

add AnyResolver

8b027ac

dr-orlovsky added the feature Feature request label Feb 13, 2024

dr-orlovsky added this to the v0.11.0 milestone Feb 13, 2024

dr-orlovsky requested review from crisdut and zoedberg February 13, 2024 15:28

This was referenced Feb 13, 2024

add AnyResolver #126

Closed

Rename esplora into esplora_blocking and do a dedicated resolver module #103

Closed

zoedberg approved these changes Feb 13, 2024

View reviewed changes

WASM fixes.

2c0e3ea

This was referenced Feb 15, 2024

Remove esplora from default features. BP-WG/bp-wallet#16

Closed

WASM fixes. #132

Merged

crisdut reviewed Feb 15, 2024

View reviewed changes

cryptoquick and others added 2 commits February 15, 2024 08:25

Switch to patching bp-wallet 0.11 branch.

b3b083f

chore: update dependencies

85b8edd

dr-orlovsky added 4 commits February 26, 2024 20:54

chore: update dependencies

84fdff0

chore: fix feature flag composition

ce03e36

Merge branch 'electrum' into HB/wasm

6b692ed

Merge pull request #132 from cryptoquick/HB/wasm

ac5c2b9

WASM fixes.

dr-orlovsky force-pushed the electrum branch from fd639e4 to ac5c2b9 Compare February 26, 2024 20:03

dr-orlovsky merged commit 87ff1bb into master Feb 26, 2024
13 of 20 checks passed

dr-orlovsky assigned zoedberg Feb 27, 2024

dr-orlovsky deleted the electrum branch September 5, 2024 11:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add electrum support #131

Add electrum support #131

dr-orlovsky commented Feb 13, 2024 •

edited

Loading

zoedberg commented Feb 13, 2024

crisdut commented Feb 13, 2024 •

edited

Loading

cryptoquick commented Feb 13, 2024 •

edited

Loading

dr-orlovsky commented Feb 15, 2024 •

edited

Loading

zoedberg commented Feb 15, 2024

cryptoquick commented Feb 15, 2024 •

edited

Loading

crisdut Feb 15, 2024

zoedberg Feb 23, 2024

zoedberg commented Feb 23, 2024

	bp-wallet = { workspace = true, features = ["fs"] }
	bp-wallet = { workspace = true, default-features = false, features = ["fs"] }

Add electrum support #131

Add electrum support #131

Conversation

dr-orlovsky commented Feb 13, 2024 • edited Loading

zoedberg commented Feb 13, 2024

crisdut commented Feb 13, 2024 • edited Loading

First Error : AnyResolver

Second Error : cli as default-member

cryptoquick commented Feb 13, 2024 • edited Loading

dr-orlovsky commented Feb 15, 2024 • edited Loading

zoedberg commented Feb 15, 2024

cryptoquick commented Feb 15, 2024 • edited Loading

crisdut Feb 15, 2024

Choose a reason for hiding this comment

zoedberg Feb 23, 2024

Choose a reason for hiding this comment

zoedberg commented Feb 23, 2024

dr-orlovsky commented Feb 13, 2024 •

edited

Loading

crisdut commented Feb 13, 2024 •

edited

Loading

cryptoquick commented Feb 13, 2024 •

edited

Loading

dr-orlovsky commented Feb 15, 2024 •

edited

Loading

cryptoquick commented Feb 15, 2024 •

edited

Loading